-
Notifications
You must be signed in to change notification settings - Fork 194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JOSM #21944 - Add 'replace selected' action to relation editor #90
base: master
Are you sure you want to change the base?
Conversation
Don't forget to open a ticket on the JOSM side and to link the ticket here and this PR there. |
Ah sorry, forgot to add the reverse link here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment with the replaceselectedright.svg
: As we are replacing something, should some portion of the blue bar be red (maybe two pixels or something?).
return filterConfirmedPrimitives(primitives, false); | ||
} | ||
|
||
protected List<OsmPrimitive> filterConfirmedPrimitives(List<OsmPrimitive> primitives, boolean abortOnSkip) throws AddAbortException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add javadocs here?
protected List<OsmPrimitive> filterConfirmedPrimitives(List<OsmPrimitive> primitives, boolean abortOnSkip) throws AddAbortException { | |
/** | |
* Filter confirmed primitives | |
* @param primitives The primitives to filter on | |
* @param abortOnSkip If the user decides to not add a primitive, abort (throw {@code AddAbortException}) | |
* @return The primitives to add to the relation. Never {@code null}, but may be an empty list. | |
* @throws AddAbortException when {@code abortOnSkip} is {@code true} <i>and</i> the user decides to not add a primitive. | |
* @since xxx | |
*/ | |
protected List<OsmPrimitive> filterConfirmedPrimitives(List<OsmPrimitive> primitives, boolean abortOnSkip) throws AddAbortException { |
You'll want to check this with ant pmd checkstyle
, but it should be right.
As a side note, I'd prefer to make this method more generic (e.g. protected <O extends IPrimitive> List<O> filterConfirmedPrimitives(List<O> primitives, boolean abortOnSkip) throws AddAbortException
), but I don't know how hard it would be to do (I suspect there would be a cascading chain of methods to change).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the javadocs. Unfortunately making the method more generic would require rewriting pretty much all of the relation editor code from OsmPrimitive
to IPrimitive
as far as I can tell from a quick glance.
6af03cb
to
0eae26d
Compare
I'm pretty sure a better icon than the one I quickly added here could be created. Unfortunately I didn't really have any good ideas how to represent replacement (and have limited art skills), so any suggestions or icon contributions are welcome. |
0eae26d
to
cadddb6
Compare
I changed the icon to be partially red and fixed some checkstyle issues |
Add a new action + button to the relation editor to replace chosen member primitives with objects from selection (while keeping the roles).